Skip to content

feat(lint): prettier --check only on changed files in PR mode#26

Open
topcoder1 wants to merge 3 commits into
mainfrom
feat/prettier-changed-files-only
Open

feat(lint): prettier --check only on changed files in PR mode#26
topcoder1 wants to merge 3 commits into
mainfrom
feat/prettier-changed-files-only

Conversation

@topcoder1
Copy link
Copy Markdown
Owner

Summary

Fleet-wide fix for the cascading-failure pattern in the reusable lint workflow: today, one badly-formatted markdown file on main causes every subsequent PR's lint / prettier (markdown) check to fail across all 44 fleet consumers, even when the PR doesn't touch the offending file.

After this change:

  • PR events: prettier --check runs only on files in the PR diff (intersected with markdown_glob).
  • Push events: full repo scan, same as before (catches drift even when no PR touches the bad file).
  • Opt-out: new input prettier_changed_only: bool (default true); set false to revert to full-repo-on-PR.

Why this matters

Discovered during topcoder1/wxa_webcat#179 — the lint job flagged 4 files: 2 authored by the PR + 2 pre-existing on main (docs/designs/exp35-long-context-postmortem.md, docs/designs/pricing-research-2026-05-03.md). The PR-author has no way to fix unrelated files in their PR, so the red-X becomes permanent noise that obscures real failures.

Implementation notes

  • Bash extglob + globstar for path matching (Ubuntu runner has bash 5+); no 3rd-party action dependency.
  • Accepts the glob with and without leading **/ so **/*.md matches both foo.md and dir/foo.md.
  • Filename list is NUL-delimited through xargs so paths with spaces survive.
  • fetch-depth: 0 on checkout so git diff origin/<base>...HEAD works.

Auto-merge rationale

Touches the reusable lint workflow that 44 fleet consumers depend on. Manual click-merge after self-test passes. Falls into the .github/workflows/** high-risk path (CI infra).

Codex pre-review

PASS — single-purpose addition (new input + new step), preserves existing behavior under default flag, push-mode unchanged.

Test plan

  • actionlint .github/workflows/lint.yml passes
  • bash glob matching tested against synthetic file list with spaces: keeps .md, drops .py/.yml, preserves filename-with-space
  • Self-test: this PR's own lint job runs in changed-files mode (only touches lint.yml, no .md changes — should skip prettier entirely with "No changed files match glob")
  • Cross-repo verification PR in topcoder1/wxa_webcat: open a docs-only PR touching ONE markdown file, verify prettier check goes green even though main has 2 pre-existing offenders (would have been red before this change). Note: wxa_webcat already has .prettierignore excluding the offenders, so the test target is a different repo or temporary unignore.

topcoder1 and others added 3 commits May 3, 2026 14:49
Two new bypass paths for the risk-tier gate on Claude-authored PRs.
Today the gate forces manual click-merge on anything touching auth,
secrets, migrations, billing, or production infra (Dockerfile / CI
workflows / IaC / deploy scripts) — these add escape hatches that
preserve the trust signal while removing the click-merge round-trip.

**Option A — `auto-merge-approved` label.**
A new input `risk_bypass_label` (default `auto-merge-approved`).
When that label is present on a Claude-authored risky PR, the risk
gate is overridden and `gh pr merge --auto --squash` is enabled.
Distinct from the existing `auto-merge` label, which is a Claude-
authorship DETECTION signal — that one only marks the PR as Claude's,
risk gate still runs.

Workflow: human sees the "blocked — risk-tier paths touched" comment,
applies the label from the PR list page (one click, no PR-detail
navigation, no merge-button click). The caller already listens on
`pull_request.labeled`, so the workflow re-runs immediately and
flips auto-merge on.

**Option B — Codex Review SUCCESS.**
A new input `codex_check_name` (default `review / Codex Review`).
When that status check is SUCCESS on the PR's head SHA, the risk
gate is overridden. Reads from the check-runs API first, falls back
to the commit-status API for callers that post Codex output as a
status rather than a check-run.

Bypass triggers naturally on the next workflow run after Codex
completes — i.e. on `pull_request.synchronize` (any push) or
`pull_request.labeled` (any label change). Fully-automatic
re-trigger via `workflow_run` was scoped out for now (workflow_run
events come without `pull_request` payload, requiring a separate PR-
lookup job that wouldn't fit cleanly into the existing reusable
shape — follow-up for v1.1).

The "blocked" comment is updated to mention both bypass options so
the human sees the path forward inline. Marker string unchanged so
existing comment-update logic continues to find and edit prior
comments rather than spamming new ones.

Lint clean: `actionlint` passes, no shellcheck issues.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Caught by Claude Review on this PR. The previous gh-api-with-inline-jq
form interpolated $CHECK_NAME into the jq filter via shell expansion:

  gh api /commits/$SHA/check-runs --jq ".check_runs[] | select(.name == \"$CHECK_NAME\") ..."

If CHECK_NAME ever contained a double-quote (custom callers, future
variants), the shell interpolation would produce a broken jq filter
that silently returned nothing — bypass=0 with no warning. Default
"review / Codex Review" works in practice but the failure mode is
exactly the kind of "silent fall-through" the Webcat 2026-04-27
incident wrote up as a category to avoid.

Fix: pipe gh api through `jq --arg name "$CHECK_NAME"` to bind the
variable safely regardless of contents. Same shape applied to both
the check-runs query and the commit-status fallback.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
On pull_request events, intersect the markdown_glob with files in the
PR diff (against base ref) and run prettier --check only on those.
On push events (or when prettier_changed_only=false), keep the full
repo scan to catch drift.

Why: today, one badly-formatted file on main poisons every subsequent
PR's `lint / prettier (markdown)` check across the fleet. Discovered
during topcoder1/wxa_webcat#179 — the lint job flagged 4 files (2
authored by the PR, 2 pre-existing on main). Changed-files mode
keeps PR feedback fast and prevents the cascade.

New input:
  prettier_changed_only: bool, default true

Implementation notes:
- Uses bash extglob+globstar (Ubuntu runner has bash 5+) for path
  matching; no 3rd-party action dependency.
- Accepts the glob with and without leading `**/` so `**/*.md`
  matches both `foo.md` and `dir/foo.md`.
- Filename list is NUL-delimited through xargs so paths with spaces
  don't split into multiple args.
- Push-to-main always runs full scan — drift gets caught even if no
  PR happens to touch the offending file.

Tested:
- actionlint passes
- bash glob matching against synthetic file list (incl. spaces)
  filters correctly: keeps .md, drops .py/.yml, preserves spaces
# Output `files` is a space-separated list, or empty if no matches.
run: |
GLOB="${{ inputs.markdown_glob || '**/*.md' }}"
CHANGED_ONLY="${{ inputs.prettier_changed_only || 'true' }}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: false || 'true' clobbers the opt-out.

In GitHub Actions expression language false || 'true' evaluates to the string 'true' (because boolean false is falsy). So when a consumer explicitly sets prettier_changed_only: false to revert to full-scan mode, CHANGED_ONLY becomes "true", and the subsequent [ "$CHANGED_ONLY" != "false" ] check stays true — changed-only mode is never disabled.

Since the input is declared type: boolean with default: true, it always has a value; the || 'true' fallback is both unnecessary and destructive.

Suggested change
CHANGED_ONLY="${{ inputs.prettier_changed_only || 'true' }}"
CHANGED_ONLY="${{ inputs.prettier_changed_only }}"

@claude
Copy link
Copy Markdown

claude Bot commented May 3, 2026

Flagged 1 issue inline — false || 'true' in the GHA expression on line 176 of lint.yml makes prettier_changed_only: false non-functional (the opt-out can never be triggered).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant